Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[UnitTests] Added cuDNN to default test targets #8383

Merged
merged 5 commits into from
Aug 9, 2021

Conversation

Lunderberg
Copy link
Contributor

@Lunderberg Lunderberg commented Jul 1, 2021

Some unit tests explicitly test cudnn in addition to tvm.testing.enabled_targets(). This moved the cudnn checks into the same framework as all other targets, and adds it to the default list of targets to be run. Also, added @tvm.testing.requires_cudnn for tests specific to cudnn.

In principle, this should pass CI, since any non-cudnn tests can fall back to the cuda implementation, but "should" is a bit of a magic word.

@Lunderberg Lunderberg force-pushed the tvm_testing_default_targets branch from 04a36d4 to 1a2838e Compare July 8, 2021 16:13
@Lunderberg
Copy link
Contributor Author

Updated to fix pylint error.

@Lunderberg Lunderberg force-pushed the tvm_testing_default_targets branch 2 times, most recently from b6fb4d9 to f57debb Compare July 12, 2021 16:14
@Lunderberg
Copy link
Contributor Author

Current failures.

  • test_conv2d_nchw doesn't support asymmetric padding on cudnn. Can add an explicit pytest.xfail on that test case once [Topi][UnitTests] Parameterize conv2d and depthwise_conv2d tests #8433 is merged.
  • test_batch_matmul errors out for dynamic batch matmul not being supported on cuda. Changed check from direct comparison with the string "cuda" to a check on the target.kind.name.

Copy link
Contributor

@comaniac comaniac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to also let a Vulkan folk review the changes in target_kind.cc.

Comment on lines 383 to 384
if len(target_names) == 0:
target_names = DEFAULT_TEST_TARGETS
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit

Suggested change
if len(target_names) == 0:
target_names = DEFAULT_TEST_TARGETS
if not target_names:
target_names = DEFAULT_TEST_TARGETS

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call, and updated.

@Lunderberg Lunderberg force-pushed the tvm_testing_default_targets branch 4 times, most recently from 80db1a3 to 4df4417 Compare July 29, 2021 15:16
@Lunderberg Lunderberg force-pushed the tvm_testing_default_targets branch from 4df4417 to 93a9535 Compare August 3, 2021 14:43
@Lunderberg
Copy link
Contributor Author

Will need rebasing on main to resolve conflicts with #8602 (merged) and #8542 (waiting on CI). #8542 is the more important one, so this PR can wait until it is done.

…arget

- Read target.kind.name instead of using string manipulation.

- Target device query on a non-existent target is no longer an error.
  This occurs if expanding `vulkan -from_device=0` on a non-GPU
  machine.
Some unit tests explicitly test cudnn in addition to
`tvm.testing.enabled_targets()`.  This moved the cudnn checks into the
same framework as all other targets, and adds it to the default list
of targets to be run.  Also, added `@tvm.testing.requires_cudnn` for
tests specific to cudnn.
@Lunderberg
Copy link
Contributor Author

Hopefully the last update needed here, rebased to main and fixed an issue with the conv2d_cudnn handling of dilation.

Previously, cuda/nvptx targets were excluded.  Changed it to look up
by target.kind.name, and to also exclude vulkan/opencl, as the dynamic
lookup currently doesn't work on those backends.
@masahi masahi merged commit 8679b4f into apache:main Aug 9, 2021
@Lunderberg Lunderberg deleted the tvm_testing_default_targets branch August 9, 2021 01:19
mehrdadh pushed a commit to mehrdadh/tvm that referenced this pull request Aug 11, 2021
* [Target][UnitTests] Look up target requirements based on tvm.target.Target

- Read target.kind.name instead of using string manipulation.

- Target device query on a non-existent target is no longer an error.
  This occurs if expanding `vulkan -from_device=0` on a non-GPU
  machine.

* [UnitTests] Added cuDNN target to default test targets

Some unit tests explicitly test cudnn in addition to
`tvm.testing.enabled_targets()`.  This moved the cudnn checks into the
same framework as all other targets, and adds it to the default list
of targets to be run.  Also, added `@tvm.testing.requires_cudnn` for
tests specific to cudnn.

* [UnitTests] pytest.xfail for CuDNN conv2d with asymmetric padding

* [Topi][CuDNN] Added handling of dilation to conv2d_cudnn

* [Topi] Skip dynamic batch matmul on cudnn, vulkan, opencl

Previously, cuda/nvptx targets were excluded.  Changed it to look up
by target.kind.name, and to also exclude vulkan/opencl, as the dynamic
lookup currently doesn't work on those backends.

Co-authored-by: Eric Lunderberg <[email protected]>
ylc pushed a commit to ylc/tvm that referenced this pull request Sep 29, 2021
* [Target][UnitTests] Look up target requirements based on tvm.target.Target

- Read target.kind.name instead of using string manipulation.

- Target device query on a non-existent target is no longer an error.
  This occurs if expanding `vulkan -from_device=0` on a non-GPU
  machine.

* [UnitTests] Added cuDNN target to default test targets

Some unit tests explicitly test cudnn in addition to
`tvm.testing.enabled_targets()`.  This moved the cudnn checks into the
same framework as all other targets, and adds it to the default list
of targets to be run.  Also, added `@tvm.testing.requires_cudnn` for
tests specific to cudnn.

* [UnitTests] pytest.xfail for CuDNN conv2d with asymmetric padding

* [Topi][CuDNN] Added handling of dilation to conv2d_cudnn

* [Topi] Skip dynamic batch matmul on cudnn, vulkan, opencl

Previously, cuda/nvptx targets were excluded.  Changed it to look up
by target.kind.name, and to also exclude vulkan/opencl, as the dynamic
lookup currently doesn't work on those backends.

Co-authored-by: Eric Lunderberg <[email protected]>
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 13, 2022
* [Target][UnitTests] Look up target requirements based on tvm.target.Target

- Read target.kind.name instead of using string manipulation.

- Target device query on a non-existent target is no longer an error.
  This occurs if expanding `vulkan -from_device=0` on a non-GPU
  machine.

* [UnitTests] Added cuDNN target to default test targets

Some unit tests explicitly test cudnn in addition to
`tvm.testing.enabled_targets()`.  This moved the cudnn checks into the
same framework as all other targets, and adds it to the default list
of targets to be run.  Also, added `@tvm.testing.requires_cudnn` for
tests specific to cudnn.

* [UnitTests] pytest.xfail for CuDNN conv2d with asymmetric padding

* [Topi][CuDNN] Added handling of dilation to conv2d_cudnn

* [Topi] Skip dynamic batch matmul on cudnn, vulkan, opencl

Previously, cuda/nvptx targets were excluded.  Changed it to look up
by target.kind.name, and to also exclude vulkan/opencl, as the dynamic
lookup currently doesn't work on those backends.

Co-authored-by: Eric Lunderberg <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants